Skip to content

fix: getPDPService always returns a pdpOffering #324

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Oct 20, 2025
Merged

fix: getPDPService always returns a pdpOffering #324

merged 2 commits into from
Oct 20, 2025

Conversation

wjmelements
Copy link
Contributor

@wjmelements wjmelements commented Oct 16, 2025 •
edited
Loading

Reviewer @rvagg
See also FilOzone/filecoin-services#295
Neither of these issues were caught before because we currently catch all exceptions in SPRegistryService.getPDPService, a bad policy.
I don't remove the catch-all because there are more problems after this; for example, 'should select a random provider when no providerId specified' will fail when trying to create the data set.

Changes

  • fix mock contract return typing
  • fix outdated mocking for test that was only working because of the type error

@wjmelements wjmelements requested a review from rvagg October 16, 2025 23:09
@github-project-automation github-project-automation bot moved this to 📌 Triage in FS Oct 16, 2025
@cloudflare-workers-and-pages Cloudflare Workers and Pages
Copy link

cloudflare-workers-and-pages bot commented Oct 16, 2025 •
edited
Loading

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs		 synapse-dev 7f8e8c3 Commit Preview URL

Branch Preview URL		 Oct 17 2025, 09:25 PM

@wjmelements
Copy link
Contributor Author

Astro build errors look unrelated:

11:09:50 PM [vite] (ssr) Error when evaluating SSR module /home/runner/work/synapse-sdk/synapse-sdk/docs/astro.config.mjs: localStorage.getItem is not a function
      at Object.<anonymous> (/home/runner/work/synapse-sdk/synapse-sdk/node_modules/.pnpm/@[email protected][email protected]/node_modules/@typescript/vfs/dist/vfs.cjs.production.min.js:1:407)
      at Module._compile (node:internal/modules/cjs/loader:1759:14)
      at Object..js (node:internal/modules/cjs/loader:1892:10)
      at Module.load (node:internal/modules/cjs/loader:1479:32)
      at Module._load (node:internal/modules/cjs/loader:1298:12)
      at TracingChannel.traceSync (node:diagnostics_channel:328:14)
      at wrapModuleLoad (node:internal/modules/cjs/loader:244:24)
      at Module.require (node:internal/modules/cjs/loader:1502:12)
      at require (node:internal/modules/helpers:152:16)
      at Object.<anonymous> (/home/runner/work/synapse-sdk/synapse-sdk/node_modules/.pnpm/@[email protected][email protected]/node_modules/@typescript/vfs/dist/index.js:5:20)
[astro] Unable to load your Astro config

@wjmelements
Copy link
Contributor Author

Astro build errors look unrelated:

11:09:50 PM [vite] (ssr) Error when evaluating SSR module /home/runner/work/synapse-sdk/synapse-sdk/docs/astro.config.mjs: localStorage.getItem is not a function
      at Object.<anonymous> (/home/runner/work/synapse-sdk/synapse-sdk/node_modules/.pnpm/@[email protected][email protected]/node_modules/@typescript/vfs/dist/vfs.cjs.production.min.js:1:407)
      at Module._compile (node:internal/modules/cjs/loader:1759:14)
      at Object..js (node:internal/modules/cjs/loader:1892:10)
      at Module.load (node:internal/modules/cjs/loader:1479:32)
      at Module._load (node:internal/modules/cjs/loader:1298:12)
      at TracingChannel.traceSync (node:diagnostics_channel:328:14)
      at wrapModuleLoad (node:internal/modules/cjs/loader:244:24)
      at Module.require (node:internal/modules/cjs/loader:1502:12)
      at require (node:internal/modules/helpers:152:16)
      at Object.<anonymous> (/home/runner/work/synapse-sdk/synapse-sdk/node_modules/.pnpm/@[email protected][email protected]/node_modules/@typescript/vfs/dist/index.js:5:20)
[astro] Unable to load your Astro config

It works locally.

rvagg
rvagg approved these changes Oct 20, 2025
View reviewed changes
Copy link
Collaborator

@rvagg rvagg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

astro error is because current Node.js became v25.0.0, so we have a new incompatibility to look at, not relevant here

@wjmelements wjmelements merged commit 42389da into master Oct 20, 2025
9 of 12 checks passed
@wjmelements wjmelements deleted the fix-sp-registry-service-test-mocking branch October 20, 2025 01:52
@github-project-automation github-project-automation bot moved this from 📌 Triage to 🎉 Done in FS Oct 20, 2025
@github-actions github-actions bot mentioned this pull request Oct 20, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rvagg rvagg rvagg approved these changes

Assignees

No one assigned

Labels

None yet

Projects

FS
Status: 🎉 Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@wjmelements @rvagg